Skip to content

[SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled#3942

Merged
marcalff merged 5 commits intoopen-telemetry:mainfrom
AyushMukkanwar:fix-config-double-negative
Mar 19, 2026
Merged

[SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled#3942
marcalff merged 5 commits intoopen-telemetry:mainfrom
AyushMukkanwar:fix-config-double-negative

Conversation

@AyushMukkanwar
Copy link
Copy Markdown
Contributor

@AyushMukkanwar AyushMukkanwar commented Mar 18, 2026 •
edited by marcalff
Loading

Fixes #3867

Changes

Replaced the private disabled_ members in TracerConfig, MeterConfig and LoggerConfig with enabled_ to avoid double-negative configuration patterns, aligning with OpenTelemetry Specification 4823.

The public configuration APIs Enabled(), Disabled(), and IsEnabled() remain fully backwards-compatible, while the class constructors were updated to default enabled=true.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added (Existing tests verify functionality via public API)
  • Changes in public API reviewed

@AyushMukkanwar AyushMukkanwar requested a review from a team as a code owner March 18, 2026 15:12
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 18, 2026 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: marcalff / name: Marc Alff (f8e1d91)

@marcalff marcalff added the pr:waiting-on-cla Waiting on CLA label Mar 18, 2026
@marcalff
Copy link
Copy Markdown
Member

marcalff commented Mar 18, 2026

Please remove the links to private files in the change set comments, like cci:2://file:///home/ayush_mukkanwar/Dev/OSS/opentelemetry-cpp/sdk/test/trace/tracer_config_test.cc:76:0-78:2

No need to link to a file name and line number, just TracerConfig is enough.

@AyushMukkanwar
Copy link
Copy Markdown
Contributor Author

AyushMukkanwar commented Mar 18, 2026

Please remove the links to private files in the change set comments, like cci:2://file:///home/ayush_mukkanwar/Dev/OSS/opentelemetry-cpp/sdk/test/trace/tracer_config_test.cc:76:0-78:2

No need to link to a file name and line number, just TracerConfig is enough.

sorry about that, just signed the cla.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (61e550e) to head (f8e1d91).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3942      +/-   ##
==========================================
- Coverage   90.06%   90.05%   -0.01%     
==========================================
  Files         226      226              
  Lines        7229     7229              
==========================================
- Hits         6510     6509       -1     
- Misses        719      720       +1
Files with missing lines Coverage Δ
sdk/include/opentelemetry/sdk/logs/logger_config.h 100.00% <100.00%> (ø)
...k/include/opentelemetry/sdk/metrics/meter_config.h 100.00% <100.00%> (ø)
...dk/include/opentelemetry/sdk/trace/tracer_config.h 100.00% <100.00%> (ø)
sdk/src/logs/logger_config.cc 100.00% <100.00%> (ø)
sdk/src/metrics/meter_config.cc 100.00% <100.00%> (ø)
sdk/src/trace/tracer_config.cc 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcalff marcalff removed the pr:waiting-on-cla Waiting on CLA label Mar 18, 2026
marcalff
marcalff reviewed Mar 18, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

Please add a CHANGELOG entry, and a new entry in the important notes section:

In TracerConfig(bool) and friends, the meaning of the bool parameter has changed, user code needs to be adjusted.

(to reword of course)

@marcalff
Copy link
Copy Markdown
Member

marcalff commented Mar 18, 2026

CI break is:

CHANGELOG.md:86:81 MD013/line-length Line length [Expected: 80; Actual: 139]
CHANGELOG.md:87:81 MD013/line-length Line length [Expected: 80; Actual: 96]

marcalff
marcalff approved these changes Mar 18, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix.

Will merge after formatting fixes in CHANGELOG.md

marcalff
marcalff reviewed Mar 18, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
@marcalff
Apply suggestions from code review
1199afa 
Co-authored-by: Marc Alff <marc.alff@free.fr>
@marcalff marcalff changed the title Fix: Swap Tracer/Meter/LoggerConfig disabled for enabled to avoid double negatives [SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled to avoid double negatives Mar 18, 2026
@marcalff marcalff changed the title [SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled to avoid double negatives [SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled Mar 18, 2026
@marcalff marcalff mentioned this pull request Mar 19, 2026
Merged
3 tasks
@marcalff marcalff merged commit ed84877 into open-telemetry:main Mar 19, 2026
68 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Mar 20, 2026
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marcalff marcalff marcalff approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[SDK] Swap Tracer/Meter/LoggerConfig disabled for enabled

3 participants

@AyushMukkanwar @marcalff @lalitb